Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace truetime with networktime #62

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

stoyicker
Copy link
Member

Doesn't lend itself to much of a split I'm afraid.

@stoyicker stoyicker requested a review from a team as a code owner July 2, 2024 11:08
@stoyicker stoyicker marked this pull request as draft July 2, 2024 11:08
@stoyicker stoyicker force-pushed the jantonio/networktime branch 2 times, most recently from b09e065 to 5445dc3 Compare July 2, 2024 12:14
@stoyicker
Copy link
Member Author

TODO Mark as ready when we release dropping the forks

@paulinaaniola
Copy link
Contributor

I see that it's a draft but when it will be ready for review could you please elaborate a bit more about this change? I see some changes to the event producer and I don’t know where it came from.

@stoyicker
Copy link
Member Author

I see that it's a draft but when it will be ready for review could you please elaborate a bit more about this change? I see some changes to the event producer and I don’t know where it came from.

You may consider this ready for code review now. The reason it's a draft is to prevent it from being merged before we've released dropping MQA, but no code changes are planned.

The changes you see in EventProducer are replacing truetime with networktime. The sleep block is to ensure initialization of the time before it gets queried. TrueTime also had this issue (it's more of a design thing really), which caused crashes. What I've done is prevent queries until they're not null in all sdks to avoid the problem, so you can see the same in Player, which makes us no longer need the custom application class that we used to get around the problem when we used truetime. This guarantees that even if an app uses either Player or EventProducer directly without having done the fencing themselves, we are not an issue for them. So there should be nothing to worry about when it comes to that.

paulinaaniola
paulinaaniola previously approved these changes Jul 4, 2024
paulinaaniola
paulinaaniola previously approved these changes Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants